test(codex): give the shutdown regressions a timeout longer than their own bounds - #649

Summary

• Fixes a latent flake in the two shutdown regression tests added by fix(codex): route resume/fork through the canonical home and bound helper shutdown (#647) #648: both ran under Vitest's default 5s per-test timeout, which is shorter than the bounds they set for themselves.
• Found by running the suite on Linux as a non-root user, which is how CI runs. This repo's Actions have never executed, so nothing had ever run these tests that way.

What Changed

test/codex-bin-wrapper.test.ts gives both shutdown tests an explicit 30s per-test timeout.

Two distinct consequences of the missing timeout, both real:

1. The SIGTERM test could exceed 5s and fail. Its budget is the wrapper's 2s graceful window plus up to 4s of polling for the helper to be reaped, plus spawn overhead — over 5s in the worst case. It passed locally only because the helper usually died fast. As a non-root user on Linux it failed reliably with Test timed out in 5000ms.

2. The 12s spawnSync bound could never report anything. fix(codex): route resume/fork through the canonical home and bound helper shutdown (#647) #648 added that bound specifically so a stuck wrapper would fail with a useful message instead of hanging the run. But Vitest killed the test at 5s first, so the message never printed. The bound was inert for its stated purpose.

Validation

Linux, non-root, default timeout (the configuration that exposed this):

• Before: FAIL ... force-stops an app helper that ignores SIGTERM (#647) → Test timed out in 5000ms
• After: passes in ~2.1s

And against the unfixed wrapper, the diagnostic now surfaces as intended:

AssertionError: wrapper never returned within 12000ms: the helper ignored SIGTERM

rather than a bare Vitest timeout.

• npm run lint
• npm run typecheck
• npm test — Windows: 5315 passed, 4 skipped, 0 failed
• Linux container, root and non-root

Risk and Rollback

• Risk: very low. Test-only; no production code touched. A longer per-test timeout cannot mask a failure — the tests still assert their own 12s bound internally, so a stuck wrapper fails at 12s with a message rather than at 30s.
• Rollback: revert the commit.

Additional Notes

Worth flagging separately: vitest.config.ts sets no testTimeout, so every test in this repo runs under the 5s default. Any other test that spawns real subprocesses is exposed to the same class of flake. This PR only fixes the two tests it introduced.

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

this pr gives two shutdown regression tests enough time to reach their own bounded failure diagnostics instead of vitest’s 5s default timeout.

• adds a shared 30s per-test timeout.
• leaves production behavior and token handling unchanged.
• preserves the windows skip for the posix-only signal test; no windows filesystem risk or concurrency issue is introduced.
• no vitest coverage is missing for this timeout-only correction.

Confidence Score: 5/5

this test-only change appears safe to merge.

the 30s outer timeout exceeds all bounded work in both shutdown tests, while their existing 12s process timeout still produces the intended diagnostic first.

_{Reviews (1): Last reviewed commit: "test(codex): give the shutdown regressio..." | Re-trigger Greptile}